Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for optional explicit BinarySuffix precision #47

Merged
merged 10 commits into from
Nov 5, 2015

Conversation

mostertb
Copy link
Contributor

When humanizing a number of bytes into the largest possible unit (as the BinarySuffix humanizer already does) you may have differing requirements in the number of decimal places you wish to show.

For example, you may which to truncate the result to a natural number if displaying the value within a paragraph or you may wish to show three decimal places if you want to still maintain information about the next largest unit.

My change gives this functionality via an optional third precision parameter to Number::binarySuffix().
The parameter being optional means that the change is non-breaking.

I have attempted to make the change in such as way that it will also be compatible with future updates to the existing hard-coded ICU 56.1 decimal formats used by BinarySuffix

Lastly, I have also updated the README to include this new functionality and provided a similar test coverage to the existing functionality.

@norberttech
Copy link
Member

Yea that is definitely something that we need, I would just like to make $precision, second, not third parameter, but this would be a BC Break. What would you say about adding new method to Number
Number::preciseBinarySuffix($number, $precision, $locale = 'en'); where $precision is not optional?

@mostertb
Copy link
Contributor Author

mostertb commented Nov 2, 2015

Thank you for the feedback. I'm glad that I could add something useful.

I happy to add a PHP Number::preciseBinarySuffix(...) function. I'll update the pull request later today

@mostertb
Copy link
Contributor Author

mostertb commented Nov 3, 2015

I've added PHP Number::preciseBinarySuffix(...)
Also updated the README and moved the tests in to phpunit

array("5.000 MB", 1048576 * 5, 3),
array("2.000 GB", 1073741824 * 2, 3),
array("3.000 TB", 1099511627776 * 3, 3),
array("1.178 PB", 1325899906842624, 3),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add some tests for precision that is bigger and lower than 3? For example, 1,2,3,4,5,6,7 ?

@mostertb
Copy link
Contributor Author

mostertb commented Nov 4, 2015

Hi Norbert

I've improved the test as requested and added cases for exceptions. It was a worthwhile exercise as I did find an issue that has been addressed in
mostertb@27a65ad

norberttech pushed a commit that referenced this pull request Nov 5, 2015
Support for optional explicit BinarySuffix precision
@norberttech norberttech merged commit 9d17321 into coduo:master Nov 5, 2015
@norberttech
Copy link
Member

@mostertb awesome, good job! Thanks a lot 🍻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants